Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Table transformer #1900

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

zachmayer
Copy link
Contributor

@zachmayer zachmayer commented Jun 7, 2024

What does this PR do?

Add support for the table transformer in the optimizer class: #1761

The table transformer does not support ORTModelForImageClassification and there's no ORTModelForObjectDetection, so I just used ORTModelForCustomTasks.

Maybe a good follow-up task would be to add ORTModelForObjectDetection and possibly also ORTModelForImageFeatureExtraction (as ORTModelForFeatureExtraction only supports text models)

I ran this on microsoft/table-transformer-detection and confirmed it optimizes the model:

Onnx model graph: 2247
Opt model graph: 1136

I see a very small speedup in inference time.
Onnx Model Time: 65.42 reps per second
Optimized Onnx Model Time: 67.34 reps per second

The optimized model binary is slightly smaller than the original model. (115.0mb vs 115.1mb)

Fixes # (issue)

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

Who can review?

@zachmayer
Copy link
Contributor Author

@mht-sharma — if you have a minute to review my PR, I would appreciate it!

Copy link
Contributor

@mht-sharma mht-sharma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zachmayer thanks for the PR. I suppose having an ORTModelForObjectDetection would be better than using ORTModelForCustomTasks since this is more generic. Could you please add a class for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants